Conversation
|
Useful tip: $ find -empty
./.git/objects/info
./.git/refs/tags
./api/net/http/parse.hpp
./src/plugins/system_log.cpp
./test/tests-integration/hw/serial/README.md
./test/tests-integration/performance/.keep
./test/tests-integration/fs/memdisk/sector0.disk
./test/tests-unit/performance/.keep
./test/tests-unit/stl/.keepAlso there's several references to conan: I just mention this since it's probably worth cleaning that up at some point. |
alfreb
left a comment
alfreb
left a comment
There was a problem hiding this comment.
Conditional approval: I prefer ./test/integration over ./test/tests-integration - besides that this is all good.
There is a case to be made for keeping tests testing the same thing next to one another, since most humans will probably find it easier to understand which tests we have for a certain set of features if they're bundled together. So you're prioritizing reorganizing this to make test.sh maintainance slightly easier - which was not very hard to begin with. Still, I don't object - it's no big deal. Just change the name.
Adding a separate category for memory tests is a big improvement - thanks for that!
There was a problem hiding this comment.
From the perspective of maintaining test.sh this makes sense. But from the perspective of working on a specific feature, or understanding what coverage we have for it, it's a bit worse. But either way is fine, so this is ok.
There was a problem hiding this comment.
I intend to make that sort of overview rather simple after refactoring test.sh (again). If we requested to test some module (say net/tcp), it will report that there were no unit tests, integration tests or performance/stres found for that unit. Not an error, but an informative message.
Personally I use find and tree a lot for this form of generic overview, but I do see your point.
There was a problem hiding this comment.
You prefer to clutter the top level directory? I was hoping to move unittests.nix into here eventually too. I can revert it if you think that's better.
test/test.sh
Outdated
| # to make sure core functionality is not broken by missing locks etc. when waking up more cores. | ||
| nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/net/udp --run ./test.py | ||
| nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/kernel/paging --run ./test.py | ||
| nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/memory/paging --run ./test.py |
There was a problem hiding this comment.
Related: I don't have a good argument, but I feel ilke kernel is a very broad term for anything test-related. Everything is a kernel test, is it not? The files under it probably should all be moved to more semantic locations.
| ${UNIT_TESTS}/util/base64.cpp | ||
| ${UNIT_TESTS}/util/bitops.cpp | ||
| ${UNIT_TESTS}/util/buddy_alloc_test.cpp | ||
| ${UNIT_TESTS}/memory/alloc/buddy_alloc_test.cpp |
There was a problem hiding this comment.
Downside to this change is that it no longer matches the structure of src/. Oh well. (Maybe that should be moved too)
| # | ||
| exclusions=( | ||
| ) | ||
| run_testsuite "${INTEGRATION_TESTS}/memory" "${exclusions[@]}" |
There was a problem hiding this comment.
Did you catch the other missing ones? I feel they should be added too, but probably best on its own PR.
|
Removed the prefix and reverted |
mazunki
force-pushed
the
ref-dirs
branch
2 times, most recently
from
9f3a85d to
382b435
Compare
|
Rebased to merge with #2316. Tests are still happy. |
|
Weird. It rebased cleanly on my end. Let's see if this helps. Running tests now. |
|
Yep, they're all fine. Maybe Github just wanted confirmation on the path changes? |
MagnusS
left a comment
MagnusS
left a comment
There was a problem hiding this comment.
Generally lgtm, but README also needs an update to refer to the new test.sh location
|
I have updated the README. One can now also actually use the |
3 participants
This PR reorganizes the
./test/subdirectory and reunites a few files which are specific to tests only into this subdirectory../test.shis now under here too.There is now a clear separation between what constitutes as a unit test, and what constitutes as an integration test at the top level. Before, this was done under each target. This will make it easier to later refactor
./test.sh.This comes with quite many atomic commits, which might seem scary at first sight; but doing it this way makes it easier to understand each change. I hope.
I am pretty confident we're running all the same tests, but I would appreciate someone double checking it. All 8 (previously 7) test sets are passing on my end.
I've noticed several tests are never actually being run:
util/{tar,tar_gz}/*,posix/*,plugin/*,mod/*,hw/*,fs/*, unless I'm missing something. This remains the same as it was.I also cannot sea any use of
/userspacenor/test/userspacewithin any of the scripts. I haven't touched these, but I have to say it's a bit confusing: what does it do, and why does it exist? Is it legacy code?In a future PR I would like to move
unittests.nixinto here too, but (while still possible) would be better done after refactoring some of the other*.nixfiles to reduce maintenance burden.